Pull Request Test Coverage Report for Build 3409071923

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 9 unchanged lines in 1 file lost coverage.
• Overall coverage remained the same at 49.969%

Totals
Change from base Build 3395015229:	0.0%
Covered Lines:	7846
Relevant Lines:	14243

💛 - Coveralls

@mweststrate has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@MuhmdRaouf has updated the pull request. You must reimport the pull request before landing.

@mweststrate Hey, this PR was focused on fixing the problem located here

as you can see you check if defined(DEBUG) || defined(FB_SONARKIT_ENABLED)
and defined(FB_SONARKIT_ENABLED) is always true due PodSpec file here

which leads Flipper to always work even in a production env because of the OR condition

I hope that makes sense so far.

so to this PR, If I make env PRODUCTION=1 npx pod-install

react-native will exclude Flipper pods from installing and when making iOS build the condition defined(FB_SONARKIT_ENABLED) will make the lib runs to look for FlipperClient.h which will cause the error

node_modules/react-native-flipper/ios/FlipperReactNativeJavaScriptPluginManager.h:12:9: 'FlipperKit/FlipperClient.h' file not found cause flipper pods haven't got installed.

so this PR prevents this error from happing as it's removing the compiler_flags = '-DFB_SONARKIT_ENABLED=1'
which will lead both conditions to false and build complete successfully.

also, I would suggest changing the condition here

#if defined(DEBUG)
#if defined(FB_SONARKIT_ENABLED)
 // logic goes here
#endif // FB_SONARKIT_ENABLED
#endif // DEBUG

or change the condition to be AND

#if defined(DEBUG) && defined(FB_SONARKIT_ENABLED)
// logic goes here
#endif // DEBUG && FB_SONARKIT_ENABLED

I would love to make that PR too, just let me know how it will go with this PR and we can go from there.

@MuhmdRaouf has updated the pull request. You must reimport the pull request before landing.

@lblasa would you mind taking a closer look on the above? thanks!

When setting :flipper_configuration => FlipperConfiguration.disabled in the Podfile, the React-native build indeed fails with
node_modules/react-native-flipper/ios/FlipperReactNativeJavaScriptPluginManager.h:12:9: 'FlipperKit/FlipperClient.h' file not found

I think this error should not occur when disabling Flipper, regardless of the environment variable (PRODUCTION) being set or not.
So I think the solution in the PR might not be the ideal solution.

In our case, our Podfile looks something like this:

:flipper_configuration => ENV['APPCENTER_BUILD_ID'] ? FlipperConfiguration.disabled : FlipperConfiguration.enabled(["Debug"], { 'Flipper' => flipperkit_version })```

@mweststrate merged this pull request in 0c442bf.